Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't throw an error in raw! if the stream is closed #56589

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Conversation

Keno
Copy link
Member

@Keno Keno commented Nov 18, 2024

This was added in #12568 to protect against a segfault after close(stdin). However, the API is not great, because the stdin closing is an asynchronous event, so there isn't really any way to use this API without inccurring an error. Further, it already returns an error code of whether or not the action suceeded, and it's bad practice to have two ways for an operation to fail. Remove the error check and handle a closed stream gracefully returning an EOF error. In all users in Base, this EOF error is ignored, but we will gracefully check for EOF later and shut down the REPL, which is the desired behavior.

Fixes timholy/Revise.jl#859

This was added in #12568 to protect against a segfault after `close(stdin)`.
However, the API is not great, because the stdin closing is an asynchronous
event, so there isn't really any way to use this API without inccurring
an error. Further, it already returns an error code of whether or not
the action suceeded, and it's bad practice to have two ways for an operation
to fail. Remove the error check and handle a closed stream gracefully returning
an EOF error. In all users in Base, this EOF error is ignored, but we will
gracefully check for EOF later and shut down the REPL, which is the desired
behavior.

Fixes timholy/Revise.jl#859
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When handle is NULL, that means the operation is in a bad (undefined) state. We don't free the memory while the container object is still around.

@vtjnash vtjnash merged commit 0de9164 into master Nov 18, 2024
5 of 7 checks passed
@vtjnash vtjnash deleted the kf/rawerror branch November 18, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"UNHANDLED TASK ERROR: TaskFailedException" when running extra tests
2 participants